Skip to content

RequiredPtr: Fix template conditional for MSVC#114222

Merged
akien-mga merged 1 commit intogodotengine:masterfrom
akien-mga:msvc-requiredptr-template
Dec 21, 2025
Merged

RequiredPtr: Fix template conditional for MSVC#114222
akien-mga merged 1 commit intogodotengine:masterfrom
akien-mga:msvc-requiredptr-template

Conversation

@akien-mga
Copy link
Copy Markdown
Member

@akien-mga akien-mga commented Dec 19, 2025

Fixes #114206.

I'm not familiar with this syntax but a quick search on the error yielded https://stackoverflow.com/a/71512930, and that seems to solve it for @isaaccp.

I have no idea why MSVC on our CI doesn't have a problem though.

@akien-mga akien-mga added this to the 4.6 milestone Dec 19, 2025
@akien-mga akien-mga added the bug label Dec 19, 2025
@akien-mga akien-mga requested a review from a team as a code owner December 19, 2025 22:41
@akien-mga akien-mga requested review from dsnopek and removed request for a team December 19, 2025 22:41
Copy link
Copy Markdown
Contributor

@dsnopek dsnopek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code change seems fine.

We use the same trick in a whole bunch of other places in this class, but not all of them. For example, if you look up at the constructors that take T_Other, we're not doing it there.

So, I'm pretty unclear on when it's required and when it isn't, and since I haven't tested on MSVC myself, there's the possibility that more of these will need to be changed?

@akien-mga
Copy link
Copy Markdown
Member Author

Yeah I have no clue either why this is needed in some cases and apparently not all. We could defensively use typename U = T everywhere then and never use T directly, but someone who understand this stuff better than I do should probably look into why this indirection is needed and why MSVC is touchy only about some of them.

Copy link
Copy Markdown
Member

@Ivorforce Ivorforce left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM
MSVC is flaky enough that we don't have to explain it. If clang and GCC are.happy, but msvc isn't, it usually means we're dealing with another obscure msvc bug. So i don't think we need to go out of our way to understand the exact cause.

@akien-mga akien-mga merged commit ef36e69 into godotengine:master Dec 21, 2025
20 checks passed
@akien-mga akien-mga deleted the msvc-requiredptr-template branch December 21, 2025 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Can't build latest master on Visual Studio (error C2938: 'std::enable_if_t<false,int>' : Failed to specialize alias template)

3 participants